Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6e1369a556
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| markInternalWrite(); | ||
| const result = await ingestAudio( |
There was a problem hiding this comment.
Move internal-write marker to the actual write point
markInternalWrite() is called before ingestAudio(), but audio ingestion performs a remote transcription that can run up to 120 seconds; the hot-reload skip window is only 10 seconds, so long transcriptions can expire the skip window before storeObservation writes. In that case the file watcher will treat this internal write as external and trigger unnecessary reload/reindex work, which can cause avoidable contention during ingest-heavy workflows.
Useful? React with 👍 / 👎.
| @@ -0,0 +1,173 @@ | |||
| import { describe, it, expect, afterEach, beforeEach } from 'bun:test'; | |||
There was a problem hiding this comment.
Use Vitest APIs in the new test file
The repository’s test command runs Vitest and includes tests/**/*.test.ts, but this new test imports from bun:test. Under the declared test runner, that import is unresolved, so CI/local npm test will fail before executing these assertions. Switching this file to Vitest imports (or globals) is necessary to keep the test suite runnable.
Useful? React with 👍 / 👎.
|
Thanks for pushing on multimodal ingestion — the capability is useful, but this one still has a couple of concrete blockers.
So I don't think this is just a test-fix PR — it needs one more pass on provider/config semantics as well. |
Summary
Adds an
memorix_ingest_audioMCP tool that transcribes audio files using the Whisper API and stores the transcript as a memorix observation.What's included
src/multimodal/audio-loader.ts— Whisper API client with:MEMORIX_WHISPER_MODEL, default:whisper-1)src/multimodal/index.ts— shared multimodal utilities (MIME detection, file validation)src/server.ts— MCP tool registration (memorix_ingest_audio)tests/multimodal/audio-loader.test.ts— 8 tests covering:Design decisions
MEMORIX_WHISPER_API_KEY(falls back toOPENAI_API_KEY). Tool returns clear error if unconfigured.how-it-worksobservation type with audio metadata in facts.fetchfor API calls andfsfor file reading.Tests